Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug Fix: Phishing Detection Dataset Discrepancies #3440

Merged
merged 3 commits into from
Nov 1, 2024

Conversation

not-a-rootkit
Copy link
Collaborator

@not-a-rootkit not-a-rootkit commented Oct 22, 2024

Task/Issue URL: https://app.asana.com/0/1204023833050360/1208567121137949/f
Tech Design URL:
CC:

Description:
In Implement desktop integration efficacy tests - 5-7 days it was discovered that Swift's client-side caching results in out-of-date datasets and significant dataset discrepancies between different clients. For example, it's very common for the same request to return different results from the backend, resulting in a client believing they are updating to a newer revision than they are. Over time, this compounds and results in disparate versions of the same dataset across different clients, putting users at risk of landing on newer phishing pages.

Fix:

  • Remove Client Side Caching in PhishingDetectionClient.swift
  • Ensure embedded dataset is used to replace the on-disk dataset when the revision of the embedded dataset > on disk dataset

Steps to test this PR:

  1. Check unit tests
  2. Change on-disk revision:
  3. echo "1650000" > "/System/Volumes/Data/Users/<user>/Library/Application Support/com.duckduckgo.macos.browser.debug/revision.txt"
  4. Build the browser
  5. Visit https://privacy-test-pages.site/security/badware/phishing.html
  6. Ensure blocked
  7. Check on-disk revision:
  8. cat "/System/Volumes/Data/Users/<user>/Library/Application Support/com.duckduckgo.macos.browser.debug/revision.txt"
  9. Should be > 1650000

Definition of Done:


Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation

Copy link
Contributor

This PR has been inactive for more than 7 days and will be automatically closed 7 days from now.

@github-actions github-actions bot added the stale label Oct 30, 2024
@github-actions github-actions bot removed the stale label Nov 1, 2024
not-a-rootkit added a commit to duckduckgo/BrowserServicesKit that referenced this pull request Nov 1, 2024
<!--
Note: This checklist is a reminder of our shared engineering
expectations.
-->

Please review the release process for BrowserServicesKit
[here](https://app.asana.com/0/1200194497630846/1200837094583426).

**Required**:

Task/Issue URL:
https://app.asana.com/0/1204023833050360/1208567121137949/f
iOS PR: duckduckgo/iOS#3469
macOS PR: duckduckgo/macos-browser#3440
What kind of version bump will this require?: Patch

**Optional**:

Tech Design URL:
CC:

**Description**:
In [Implement desktop integration efficacy tests - 5-7
days](https://app.asana.com/0/1207943168535188/1207205745934704/f) it
was discovered that Swift's client-side caching results in out-of-date
datasets and significant dataset discrepancies between different
clients. For example, it's very common for the same request to return
different results from the backend, resulting in a client believing they
are updating to a newer revision than they are. Over time, this
compounds and results in disparate versions of the same dataset across
different clients, putting users at risk of landing on newer phishing
pages.

Fix: 
 - Remove Client Side Caching in PhishingDetectionClient.swift
- Ensure embedded dataset is used to replace the on-disk dataset when
the revision of the embedded dataset > on disk dataset
<!--
Tagging instructions
If this PR isn't ready to be merged for whatever reason it should be
marked with the `DO NOT MERGE` label (particularly if it's a draft)
If it's pending Product Review/PFR, please add the `Pending Product
Review` label.

If at any point it isn't actively being worked on/ready for
review/otherwise moving forward (besides the above PR/PFR exception)
strongly consider closing it (or not opening it in the first place). If
you decide not to close it, make sure it's labelled to make it clear the
PRs state and comment with more information.
-->

**Steps to test this PR**:
1. Check unit tests
3. Change on-disk revision:
4. `echo "1650000" >
"/System/Volumes/Data/Users/<user>/Library/Application
Support/com.duckduckgo.macos.browser.debug/revision.txt"`
5. Build the browser
6. Visit https://privacy-test-pages.site/security/badware/phishing.html
7. Ensure blocked
8. Check on-disk revision:
9. `cat "/System/Volumes/Data/Users/<user>/Library/Application
Support/com.duckduckgo.macos.browser.debug/revision.txt"`
10. Should be > 1650000


<!--
Before submitting a PR, please ensure you have tested the combinations
you expect the reviewer to test, then delete configurations you *know*
do not need explicit testing.

Using a simulator where a physical device is unavailable is acceptable.
-->

**OS Testing**:

* [ ] iOS 14
* [ ] iOS 15
* [ ] iOS 16
* [ ] macOS 10.15
* [ ] macOS 11
* [ ] macOS 12

---
###### Internal references:
[Software Engineering
Expectations](https://app.asana.com/0/59792373528535/199064865822552)
[Technical Design
Template](https://app.asana.com/0/59792373528535/184709971311943)
@not-a-rootkit not-a-rootkit merged commit 9280027 into main Nov 1, 2024
18 checks passed
@not-a-rootkit not-a-rootkit deleted the tespach/fix-outdated-phishing-datasets branch November 1, 2024 11:41
samsymons added a commit that referenced this pull request Nov 1, 2024
…the-os

# By Dax the Duck (3) and others
# Via Dax the Duck (1) and others
* main:
  Bump version to 1.112.0 (296)
  Marking latest Bitwarden versions as incompatible (#3492)
  Bump version to 1.112.0 (295)
  Update to subscription cookie (#3489)
  Bug Fix: Phishing Detection Dataset Discrepancies (#3440)
  Bump version to 1.112.0 (294)
  Fix crash when opening permission popover for NewTab page address bar (#3484)
  update UI test (#3469)

# Conflicts:
#	DuckDuckGo.xcodeproj/project.pbxproj
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
samsymons added a commit that referenced this pull request Nov 4, 2024
# By Dax the Duck (5) and others
# Via GitHub (4) and Michal Smaga (1)
* main: (24 commits)
  Validate VPN errors before re-throwing them (#3490)
  Allowing users to delete suggestions (#3430)
  Bump version to 1.112.0 (296)
  Marking latest Bitwarden versions as incompatible (#3492)
  Bump version to 1.112.0 (295)
  Update to subscription cookie (#3489)
  Bug Fix: Phishing Detection Dataset Discrepancies (#3440)
  Bump version to 1.112.0 (294)
  Fix crash when opening permission popover for NewTab page address bar (#3484)
  Set version_check_wait_retry_limit to 1 (#3488)
  Refactor automatic update flow to use custom Sparkle user driver (#3274)
  Add to Dock - Update BSK version (#3479)
  Freemium PIR: Manual Removal Links (#3466)
  Fix Sync E2E tests (#3486)
  Fix crash on empty bookmarks html root element (#3482)
  update UI test (#3469)
  Update BSK with autofill 15.1.0 (#3480)
  Bump version to 1.112.0 (293)
  New tagline (#3401)
  add system info to webkit termination validation (#3473)
  ...

# Conflicts:
#	DuckDuckGo.xcodeproj/project.pbxproj
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants